Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defer downstream filter execution if no OAuth2AuthorizedClient is found #6719

Closed
wants to merge 1 commit into from
Closed

Defer downstream filter execution if no OAuth2AuthorizedClient is found #6719

wants to merge 1 commit into from

Conversation

philsttr
Copy link
Contributor

Prior to this change, ServerOAuth2AuthorizedClientExchangeFilterFunction would invoke next.exchange:

  • first at assembly time inside the .switchIfEmpty call.
  • second at execution time inside .flatMap when a OAuth2AuthorizedClient is found.

While this double-call should not technically cause any functional problems, since the Mono returned by the first call will not be subscribed if a OAuth2AuthorizedClient is found,
it does result in a lot of unnecessary execution and object creation. There is no technical need to invoke the downstream filters twice.

This change defers the call inside .switchIfEmpty, so that it will only execute at execution time if an OAuth2AuthorizedClient is not found.

After this change, ServerOAuth2AuthorizedClientExchangeFilterFunction will not invoke next.exchange at assembly time, and will only execute next.exchange once per subscription at execution time.

Please backport this change to 5.1.x if possible

Prior to this change, ServerOAuth2AuthorizedClientExchangeFilterFunction would invoke next.exchange:
- first at assembly time inside the .switchIfEmpty call.
- second at execution time inside .flatMap when a OAuth2AuthorizedClient is found.

While this double-call should not technically cause any functional problems, since the Mono returned by the first call will not be subscribed if a OAuth2AuthorizedClient is found,
it does result in a lot of unnecessary execution and object creation.  There is no technical need to invoke the downstream filters twice.

This change defers the call inside .switchIfEmpty, so that it will only execute at execution time if an OAuth2AuthorizedClient is not found.

After this change, ServerOAuth2AuthorizedClientExchangeFilterFunction will not invoke next.exchange at assembly time, and will only execute next.exchange once per subscription at execution time.
@jgrandja jgrandja self-assigned this Apr 1, 2019
@jgrandja jgrandja added type: enhancement A general enhancement Reactive in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) labels Apr 1, 2019
@jgrandja jgrandja added this to the 5.2.0.M2 milestone Apr 1, 2019
@jgrandja jgrandja closed this in 9593f9c Apr 1, 2019
jgrandja pushed a commit that referenced this pull request Apr 1, 2019
Prior to this change, ServerOAuth2AuthorizedClientExchangeFilterFunction would invoke next.exchange:
- first at assembly time inside the .switchIfEmpty call.
- second at execution time inside .flatMap when a OAuth2AuthorizedClient is found.

While this double-call should not technically cause any functional problems, since the Mono returned by the first call will not be subscribed if a OAuth2AuthorizedClient is found,
it does result in a lot of unnecessary execution and object creation.  There is no technical need to invoke the downstream filters twice.

This change defers the call inside .switchIfEmpty, so that it will only execute at execution time if an OAuth2AuthorizedClient is not found.

After this change, ServerOAuth2AuthorizedClientExchangeFilterFunction will not invoke next.exchange at assembly time, and will only execute next.exchange once per subscription at execution time.

Fixes gh-6719
@jgrandja
Copy link
Contributor

jgrandja commented Apr 1, 2019

Thanks for the PR @philsttr. I also back-ported to 5.1.x #6729.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants